-
-
Notifications
You must be signed in to change notification settings - Fork 601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Config get_body_args #8737
Config get_body_args #8737
Conversation
WalkthroughThe recent updates across SickChill involve a comprehensive reorganization of import statements and minor tweaks in functionality. This includes changes to metadata providers, view configurations, and template adjustments, aimed at enhancing code maintainability and clarity. Additionally, there are minor logical adjustments and method enhancements to improve the software's robustness and user experience. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sickchill/views/config/search.py (2 hunks)
Additional comments: 1
sickchill/views/config/search.py (1)
- 38-118: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [30-115]
The refactoring of the
saveSearch
function to directly retrieve parameters from the request body and update settings accordingly is a significant improvement in simplifying the method and improving parameter handling. However, there are several areas that could benefit from further refinement:
Error Handling and User Feedback: The method for accumulating errors in a list and then displaying them to the user is a good practice. However, consider enhancing the clarity of error messages to make them more actionable for the user. For instance, specifying why a directory could not be created might help users resolve the issue more efficiently.
Security Considerations: Ensure that all user inputs retrieved through
get_body_argument
are properly sanitized and validated, especially before using them in operations like directory creation or settings updates. While the PR description mentions improvements in handling request body arguments, explicit validation or sanitization steps in the code would reinforce security.Performance and Efficiency: The function updates a large number of settings individually. If possible, consider batching these updates or optimizing the settings update mechanism to minimize the performance impact, especially if this function is called frequently.
Code Readability and Maintainability: The function is quite lengthy, which could impact readability and maintainability. Consider breaking down the function into smaller, more focused sub-functions or methods. For example, handling settings related to different services (e.g., SABnzbd, NZBGet, Torrents) in separate methods could make the code more organized and easier to maintain.
Consistency with Project Standards: Ensure that the changes align with the coding standards and practices of the SickChill project, including naming conventions, error handling patterns, and documentation requirements.
Overall, the refactoring effort is commendable for its focus on simplifying parameter handling and improving the maintainability of the code. With some additional refinements, the
saveSearch
function can be further optimized for security, performance, and readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sickchill/views/config/post_processing.py (2 hunks)
Additional comments: 4
sickchill/views/config/post_processing.py (4)
- 44-44: The logic for enabling unpacking based on the
unpack
argument and the result ofself.isRarSupported()
is clear and straightforward. However, it's important to ensure that theisRarSupported
method accurately reflects the system's capability to handle RAR files. Misidentification could lead to user confusion or data loss if unpacking is attempted without proper support. Consider adding more detailed logging or user feedback mechanisms to communicate the availability of unpacking features.- 52-52: When handling the change to the unpack directory, the error message "Unable to change unpack directory to ..., check the logs." is helpful for troubleshooting. However, it might be beneficial to provide more specific guidance or suggestions for common issues directly in the error message, if possible. This could improve the user experience by reducing the need to consult logs for common configuration errors.
- 96-109: The validation and application of naming patterns for normal, anime, air-by-date, and sports configurations are crucial for the application's functionality. The use of
self.isNamingValid
to check the validity of these patterns before applying them is a good practice. However, ensure that the validation logic withinisNamingValid
is comprehensive and accounts for all possible edge cases. Additionally, providing more detailed feedback to the user when a naming configuration is invalid could help in troubleshooting configuration issues.- 82-87: The handling of metadata settings for various providers (KODI, MediaBrowser, etc.) and the subsequent configuration of these providers is a significant improvement in terms of code organization and readability. Ensure that the
set_config
method of each metadata provider properly handles the configuration data, especially in cases where the data might be incomplete or improperly formatted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sickchill/views/config/post_processing.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- sickchill/views/config/post_processing.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Actionable comments outside the diff hunks (6)
sickchill/oldbeard/failedProcessor.py (1)
Line range hint
40-40
: Undefined name_
used in logging.- self._log(_("Warning: unable to find a valid release name."), logger.WARNING) + self._log("Warning: unable to find a valid release name.", logger.WARNING)sickchill/oldbeard/show_name_helpers.py (1)
Line range hint
142-142
: Undefined name_
used in logging.- logger.info(_("Using release for release name.")) + logger.info("Using release for release name.")sickchill/oldbeard/tvcache.py (1)
Line range hint
375-375
: Undefined name_
used in logging.- logger.debug(_("Added RSS item: [{name}] to cache: {provider}").format(name=name, provider=self.provider_id)) + logger.debug("Added RSS item: [{name}] to cache: {provider}".format(name=name, provider=self.provider_id))sickchill/providers/GenericProvider.py (3)
Line range hint
41-44
: Undefined names_
used in ProviderStatus dictionary.- PROVIDER_BROKEN: _("Not working"), - PROVIDER_DAILY: _("Daily/RSS only"), - PROVIDER_BACKLOG: _("Backlog/Manual Search only"), - PROVIDER_OK: _("Daily/RSS and Backlog/Manual Searches working"), + PROVIDER_BROKEN: "Not working", + PROVIDER_DAILY: "Daily/RSS only", + PROVIDER_BACKLOG: "Backlog/Manual Search only", + PROVIDER_OK: "Daily/RSS and Backlog/Manual Searches working",
Line range hint
288-288
: Undefined name_
used in logging.- logger.debug(_("Ignoring result {title}.").format(title=title)) + logger.debug("Ignoring result {title}.".format(title=title))
Line range hint
291-291
: Undefined name_
used in logging.- logger.debug(_("Found result {title} at {url}.").format(title=title, url=url)) + logger.debug("Found result {title} at {url}.".format(title=title, url=url))
Functionality of Validators checked at 0.22.0 and 0.28.0 - functions the same in regard Exceptions with stricter checking of strings in 28 per discord discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Actionable comments outside the diff hunks (62)
sickchill/adba/aniDBtvDBmaper.py (2)
Line range hint
23-23
: Remove the unused local variableerror
to clean up the code.- except ValueError as error: + except ValueError:
Line range hint
35-35
: The variabledefaultSeason
is assigned but never used. Consider removing it or using it as intended.- defaultSeason = int(anime.get("defaulttvdbseason", 1))
sickchill/views/config/shares.py (1)
Line range hint
16-16
: The localization function_
is used but not defined in this file. Ensure it is imported from the appropriate module to avoid runtime errors.+ from sickchill.views.common import _
Also applies to: 17-17, 50-50
sickchill/oldbeard/dailysearcher.py (1)
Line range hint
26-26
: The use of_
for string localization appears throughout the code, but it is not defined or imported. This will cause runtime errors when these lines are executed.+ from sickchill.translations import _
Also applies to: 56-56, 70-70, 72-72, 77-77, 83-83, 95-95
sickchill/views/config/subtitles.py (1)
Line range hint
21-21
: The use of_
for string localization appears throughout the code, but it is not defined or imported. This will cause runtime errors when these lines are executed.+ from sickchill.translations import _
Also applies to: 22-22, 73-73
sickchill/views/manage/searches.py (1)
Line range hint
36-36
: The use of_
for string localization appears throughout the code, but it is not defined or imported. This will cause runtime errors when these lines are executed.+ from sickchill.translations import _
Also applies to: 37-37, 48-48, 57-57, 66-66, 75-75, 84-84
sickchill/oldbeard/nzbget.py (3)
Line range hint
36-36
: The logging statement uses an undefined name_
. This will cause a runtime error. Replace it with a direct string message or ensure that the_
function is defined/imported if it's intended for localization.- logger.warning(_("No NZBget host found in configuration. Please configure it.")) + logger.warning("No NZBget host found in configuration. Please configure it.")
Line range hint
157-157
: Similar to the previous comment, the logging statement uses an undefined name_
. Replace it with a direct string message.- logger.warning(_("NZBget could not add {name} to the queue").format(name=f"{result.name}.nzb")) + logger.warning("NZBget could not add {name} to the queue".format(name=f"{result.name}.nzb"))
Line range hint
160-160
: Again, the logging statement uses an undefined name_
. Replace it with a direct string message.- logger.warning(_("Connect Error to NZBget: could not add {name} to the queue").format(name=f"{result.name}.nzb")) + logger.warning("Connect Error to NZBget: could not add {name} to the queue".format(name=f"{result.name}.nzb"))sickchill/movies.py (1)
Line range hint
48-48
: Using a bareexcept
statement is not recommended as it can catch unexpected exceptions and hide programming errors. Specify the exception types that you expect to handle.- except: + except Exception as e: + logger.error("An error occurred: " + str(e))sickchill/views/config/providers.py (6)
Line range hint
27-27
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
28-28
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
58-58
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
60-60
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
62-62
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
269-269
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
sickchill/oldbeard/properFinder.py (2)
Line range hint
29-29
: Ensure localization function_
is defined or imported.+ from sickchill.views.common import _
Line range hint
259-259
: Remove unnecessary f-string as it contains no placeholders.- main_db_con.action(f"UPDATE info SET last_proper_search = ? WHERE 1", [str(when)]) + main_db_con.action("UPDATE info SET last_proper_search = ? WHERE 1", [str(when)])sickchill/oldbeard/scene_exceptions.py (1)
Line range hint
237-237
: The log statement here uses an f-string but does not include any placeholders, which is unnecessary. Consider using a regular string instead.- logger.info(f"Checking for scene exception updates from sickchill.github.io") + logger.info("Checking for scene exception updates from sickchill.github.io")sickchill/oldbeard/image_cache.py (5)
Line range hint
89-89
: The log statement here uses an f-string without placeholders, which is unnecessary. Consider using a regular string instead.- logger.debug(f"Checking if file poster_path exists") + logger.debug("Checking if file poster_path exists")
Line range hint
97-97
: Similar to the previous comment, this log statement should not use an f-string since it does not contain any placeholders.- logger.debug(f"Checking if file banner_path exists") + logger.debug("Checking if file banner_path exists")
Line range hint
105-105
: Again, this log statement should use a regular string instead of an f-string as there are no placeholders.- logger.debug(f"Checking if file fanart_path exists") + logger.debug("Checking if file fanart_path exists")
Line range hint
113-113
: This log statement incorrectly uses an f-string. It should be changed to a regular string.- logger.debug(f"Checking if file poster_thumb_path exists") + logger.debug("Checking if file poster_thumb_path exists")
Line range hint
121-121
: This is another instance where an f-string is used unnecessarily. Changing it to a regular string is recommended.- logger.debug(f"Checking if file banner_thumb_path exists") + logger.debug("Checking if file banner_thumb_path exists")sickchill/oldbeard/tvcache.py (1)
Line range hint
374-374
: The use of the_
function for localization is not defined in this file. This could lead to a runtime error if not addressed.- logger.debug(_("Added RSS item: [{name}] to cache: {provider}").format(name=name, provider=self.provider_id)) + # TODO: Define the `_` function for localization or remove if not neededsickchill/oldbeard/common.py (1)
Line range hint
33-40
: The_
function, used for internationalization, is referenced multiple times but not defined or imported in this file. This will cause a runtime error if these lines are executed. Consider importing_
from the appropriate module at the beginning of the file.+ from sickchill.translation import _
sickchill/views/config/notifications.py (4)
Line range hint
22-22
: The function_
used for localization is undefined. Ensure that the localization function is properly imported or defined to avoid runtime errors.+ from sickchill.helper import _
Line range hint
23-23
: The function_
used for localization is undefined. Ensure that the localization function is properly imported or defined to avoid runtime errors.+ from sickchill.helper import _
Line range hint
272-272
: The function_
used for localization is undefined. Ensure that the localization function is properly imported or defined to avoid runtime errors.+ from sickchill.helper import _
Line range hint
274-274
: The function_
used for localization is undefined. Ensure that the localization function is properly imported or defined to avoid runtime errors.+ from sickchill.helper import _
sickchill/providers/metadata/generic.py (9)
Line range hint
724-724
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
727-727
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
739-739
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
756-756
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
761-761
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
773-773
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
776-776
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
785-785
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.
Line range hint
792-792
: The_
function used for localization is undefined. Ensure that it is properly imported from a localization library or module to avoid runtime errors.sickchill/oldbeard/config.py (2)
Line range hint
179-179
: The functionchange_unpack_dir
uses_
which is undefined. This is typically used for localization and suggests a missing import from a localization library.+ from sickchill.localization import _
Line range hint
550-550
: The functionchange_log_dir
uses_
which is undefined, indicating a missing import for localization.+ from sickchill.localization import _
sickchill/oldbeard/postProcessor.py (17)
Line range hint
112-127
: The use of_
for string translations is not defined in this scope, which will cause aNameError
at runtime. If_
is intended as a translation function, ensure it is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
198-261
: Multiple instances of the undefined name_
used for string translations are found in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
282-327
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
347-400
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
407-438
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
459-469
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
530-637
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
701-736
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
766-791
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
860-905
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
917-959
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
982-1020
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
1031-1061
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
1072-1105
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
1114-1164
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
1189-1232
: The undefined name_
used for string translations is found multiple times in this method. This will cause aNameError
at runtime. Ensure that the translation function is properly imported and available in this file.+ from sickchill.translations import _
Line range hint
1238-1238
: The debug log statement uses an f-string without placeholders, which is unnecessary. Consider simplifying this statement.- logger.debug(f"Trying a new way to verify if we can parse this file") + logger.debug("Trying a new way to verify if we can parse this file")sickchill/tv.py (4)
Line range hint
556-556
: Undefined name_
used for string formatting, which may cause a runtime error.- logger.debug(_("{show_id}: Loading all episodes from {indexer_name}...").format(show_id=self.indexerid, indexer_name=self.indexer_name)) + logger.debug(f"{self.indexerid}: Loading all episodes from {self.indexer_name}...")
Line range hint
1933-1933
: Undefined name_
used in logging, replace with direct string formatting.- logger.debug(_("Deleting {show} {ep} from the DB").format(show=self.show.name, ep=episode_num(self.season, self.episode))) + logger.debug(f"Deleting {self.show.name} {episode_num(self.season, self.episode)} from the DB")
Line range hint
1937-1937
: Replace undefined_
function with direct string formatting in logging.- logger.debug(_("Removing myself from my show's list")) + logger.debug("Removing myself from my show's list")
Line range hint
1941-1941
: Correct the use of undefined_
in logging statements.- logger.debug(_("Deleting myself from the database")) + logger.debug("Deleting myself from the database")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Actionable comments outside the diff hunks (4)
sickchill/adba/aniDBtvDBmaper.py (1)
Line range hint
35-35
: The variabledefaultSeason
is assigned but never used. Consider removing it or implementing its intended use if it was part of an incomplete feature.- defaultSeason = int(anime.get("defaulttvdbseason", 1))
sickchill/oldbeard/properFinder.py (1)
Line range hint
29-29
: The_
function used for localization is not defined in this file. Ensure it is imported from the appropriate localization module to avoid runtime errors.+ from sickchill.translations import _
Also applies to: 41-41, 59-59, 64-64, 67-67, 74-74, 79-79, 93-93, 100-100, 104-104, 108-108, 129-129, 134-134, 163-163, 179-179, 207-207, 220-220, 251-251
sickchill/oldbeard/image_cache.py (1)
Line range hint
97-97
: Simplify the debug statements by removing unnecessary f-string usage where there are no placeholders.- logger.debug(f"Checking if file banner_path exists") + logger.debug("Checking if file banner_path exists") - logger.debug(f"Checking if file fanart_path exists") + logger.debug("Checking if file fanart_path exists") - logger.debug(f"Checking if file poster_thumb_path exists") + logger.debug("Checking if file poster_thumb_path exists") - logger.debug(f"Checking if file banner_thumb_path exists") + logger.debug("Checking if file banner_thumb_path exists")Also applies to: 105-105, 113-113, 121-121
sickchill/oldbeard/postProcessor.py (1)
Line range hint
112-112
: The function_
used for string localization is not defined. Typically, this is imported from a module likegettext
. Please ensure that the necessary import statement is added to support localization.+ from gettext import gettext as _
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 29
Actionable comments outside the diff hunks (40)
sickchill/views/news.py (1)
Line range hint
18-29
: Undefined name_
used in multiple places. Ensure that the translation function_
is properly imported or defined.+ from sickchill.views.common import _
sickchill/views/changelog.py (1)
Line range hint
19-26
: Undefined name_
used in multiple places. Ensure that the translation function_
is properly imported or defined.+ from sickchill.views.common import _
sickchill/views/authentication.py (1)
Line range hint
16-29
: Undefined name_
used in multiple places. Ensure that the translation function_
is properly imported or defined.+ from sickchill.views.common import _
sickchill/views/config/backup.py (2)
Line range hint
21-21
: The use of_
suggests a missing import for a translation function. Ensure that the translation function is properly imported to avoid runtime errors.+ from sickchill.views.common import _
Line range hint
22-22
: Similar to the previous comment, ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
sickchill/views/logs.py (6)
Line range hint
17-17
: The use of_
suggests a missing import for a translation function. Ensure that the translation function is properly imported to avoid runtime errors.+ from sickchill.views.common import _
Line range hint
23-23
: Similar to previous comments, ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
38-38
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
39-39
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
73-73
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
74-74
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
sickchill/views/imageSelector.py (4)
Line range hint
5-5
: The importsickchill.settings
is unused in this file. Consider removing it to clean up the code.- from sickchill import settings
Line range hint
22-22
: The use of_
suggests a missing import for a translation function. Ensure that the translation function is properly imported to avoid runtime errors.+ from sickchill.views.common import _
Line range hint
26-26
: Similar to previous comments, ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
48-48
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
sickchill/views/movies.py (9)
Line range hint
19-19
: The use of_
suggests a missing import for a translation function. Ensure that the translation function is properly imported to avoid runtime errors.+ from sickchill.views.common import _
Line range hint
20-20
: Similar to previous comments, ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
37-37
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
38-38
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
68-68
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
74-74
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
85-85
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
89-89
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
90-90
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
sickchill/views/history.py (8)
Line range hint
63-63
: The use of_
suggests a missing import for a translation function. Ensure that the translation function is properly imported to avoid runtime errors.+ from sickchill.views.common import _
Line range hint
64-64
: Similar to previous comments, ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
65-65
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
73-73
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
74-74
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
88-88
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
95-95
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
Line range hint
102-102
: Ensure the translation function_
is imported to prevent undefined name errors.+ from sickchill.views.common import _
sickchill/views/config/general.py (1)
Line range hint
23-23
: Ensure the_
function for internationalization is defined or imported.+ from sickchill.helper import _
sickchill/adba/__init__.py (1)
Line range hint
23-23
:MyListDelCommand
is imported but unused; consider removing it or adding it to__all__
.- from sickchill.adba.aniDBcommands import ( - AnimeCommand, - AuthCommand, - BuddyAcceptCommand, - BuddyAddCommand, - BuddyDelCommand, - BuddyDenyCommand, - BuddyListCommand, - BuddyStateCommand, - EncryptCommand, - EpisodeCommand, - FileCommand, - GroupCommand, - GroupstatusCommand, - LogoutCommand, - MyListAddCommand, - MyListCommand, - MyListDelCommand, - MyListStatsCommand, - Notification, - NotifyAckCommand, - NotifyAddCommand, - NotifyCommand, - NotifyDelCommand, - NotifyGetCommand, - NotifyListCommand, - PingCommand, - ProducerCommand, - PushAckCommand, - PushCommand, - RandomAnimeCommand, - SendMsgCommand, - UptimeCommand, - UserCommand, - VersionCommand, - VoteCommand, - ) + from sickchill.adba.aniDBcommands import ( + AnimeCommand, + AuthCommand, + BuddyAcceptCommand, + BuddyAddCommand, + BuddyDelCommand, + BuddyDenyCommand, + BuddyListCommand, + BuddyStateCommand, + EncryptCommand, + EpisodeCommand, + FileCommand, + GroupCommand, + GroupstatusCommand, + LogoutCommand, + MyListAddCommand, + MyListCommand, + MyListStatsCommand, + Notification, + NotifyAckCommand, + NotifyAddCommand, + NotifyCommand, + NotifyDelCommand, + NotifyGetCommand, + NotifyListCommand, + PingCommand, + ProducerCommand, + PushAckCommand, + PushCommand, + RandomAnimeCommand, + SendMsgCommand, + UptimeCommand, + UserCommand, + VersionCommand, + VoteCommand, + )sickchill/views/home.py (6)
Line range hint
74-74
: The function_
is used but not defined or imported, which will cause a runtime error.- return None, _("Invalid show parameters") + return None, "Invalid show parameters" # Consider defining the `_` function or importing it if it's meant for localization.
Line range hint
79-79
: The function_
is used but not defined or imported, which will cause a runtime error.- return None, _("Invalid show parameters") + return None, "Invalid show parameters" # Consider defining the `_` function or importing it if it's meant for localization.
Line range hint
86-86
: The function_
is used but not defined or imported, which will cause a runtime error.- return None, _("Invalid parameters") + return None, "Invalid parameters" # Consider defining the `_` function or importing it if it's meant for localization.
Line range hint
89-89
: The function_
is used but not defined or imported, which will cause a runtime error.- return None, _("Episode couldn't be retrieved") + return None, "Episode couldn't be retrieved" # Consider defining the `_` function or importing it if it's meant for localization.
Line range hint
118-119
: The function_
is used but not defined or imported, which will cause a runtime error.- title=_("Home"), - header=_("Show List"), + title="Home", + header="Show List" # Consider defining the `_` function or importing it if it's meant for localization.
Line range hint
196-197
: The function_
is used but not defined or imported, which will cause a runtime error.- title=_("Home"), - header=_("Show List"), + title="Home", + header="Show List" # Consider defining the `_` function or importing it if it's meant for localization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Actionable comments outside the diff hunks (4)
tests/sickchill_tests/helper/test_common.py (1)
Line range hint
343-379
: The dictionary key literals in thetest_replace_extension
function are repeated multiple times, which could lead to unexpected behavior or logical errors. Consider removing the duplicate entries to ensure the dictionary behaves as expected.- 342-343: - 343-344: - 347-348: - 348-349: - 349-350: - 350-351: - 354-355: - 355-356: - 356-357: - 357-358: - 361-362: - 362-363: - 363-364: - 364-365: - 368-369: - 369-370: - 370-371: - 371-372: - 375-376: - 376-377: - 377-378: - 378-379:sickchill/oldbeard/show_queue.py (3)
Line range hint
5-5
: The importstyping.overload
andtyping.Tuple
are unused in this file. Consider removing these to clean up the code.- from typing import overload, Tuple, Union + from typing import Union
Line range hint
211-217
: The function_
used for localization is undefined in this file. Ensure that you import or define this function to avoid runtime errors.+ from django.utils.translation import gettext as _
Line range hint
300-405
: Several instances of the function_
used for localization are undefined. This could lead to runtime errors. Make sure to import or define this function.+ from django.utils.translation import gettext as _
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Actionable comments outside the diff hunks (4)
sickchill/oldbeard/show_queue.py (4)
Line range hint
211-217
: Localization function_
is undefined in multiple places. This could cause runtime errors.+ from django.utils.translation import gettext as _
Line range hint
300-300
: Undefined localization function_
. Ensure it is properly imported.+ from django.utils.translation import gettext as _
Line range hint
328-328
: Undefined localization function_
. Ensure it is properly imported.+ from django.utils.translation import gettext as _
Line range hint
338-339
: Undefined localization function_
. Ensure it is properly imported.+ from django.utils.translation import gettext as _
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Out of diff range and nitpick comments (8)
sickchill/oldbeard/databases/movie.py (2)
Line range hint
9-9
: Remove unused importsqlalchemy.ext.declarative.declarative_base
.- from sqlalchemy.ext.declarative import declarative_base
163-163
: Ensure proper logging for unmatched movie results.Consider enhancing the log message to include more details about why the result does not match any movies, which could be helpful for debugging.
sickchill/oldbeard/providers/thepiratebay.py (2)
Line range hint
140-140
: Add missing translation function_
.+ from sickchill.helper.common import _
147-147
: Clarify the condition for trusted results.Consider adding a comment to explain the condition checking for 'trusted' and 'vip' status, as it might not be immediately clear to someone unfamiliar with the context.
SickChill.py (2)
Line range hint
47-54
: Organize module-level imports at the top of the file.- from sickchill.helper.argument_parser import SickChillArgumentParser - from sickchill.oldbeard import db, name_cache, network_timezones - from sickchill.oldbeard.event_queue import Events - from sickchill.tv import TVShow - from sickchill.update_manager import PipUpdateManager, UpdateManager - from sickchill.views.server_settings import SCWebServer + # Move these imports to the top of the file
Line range hint
198-198
: Remove unnecessary f-string.- self.log(f"{parser_name}: {result}") + self.log(f"{parser_name}: {result}")sickchill/oldbeard/helpers.py (1)
Line range hint
315-315
: The function_
for localization is not defined in this scope.+ from sickchill.translations import _
sickchill/views/api/webapi.py (1)
Line range hint
1481-1481
: Remove or define the localization function_
used in the logging statement.- logger.info(_("API: sc.backup backing up to {location}").format(location=self.location)) + logger.info("API: sc.backup backing up to {location}".format(location=self.location))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Out of diff range and nitpick comments (18)
SickChill.py (2)
Line range hint
42-49
: Move module-level imports to the top of the file to comply with PEP 8 standards.+ import mimetypes + from operator import attrgetter + from pathlib import Path + from typing import List, Union + from configobj import ConfigObj + from sickchill.helper.argument_parser import SickChillArgumentParser + from sickchill.oldbeard import db, name_cache, network_timezones + from sickchill.oldbeard.event_queue import Events + from sickchill.tv import TVShow + from sickchill.update_manager import PipUpdateManager, UpdateManager + from sickchill.views.server_settings import SCWebServer
Line range hint
193-193
: Remove the f-string as it is unnecessary when there are no placeholders.- logger.info(f"Starting SickChill [{version}] using '{config}'") + logger.info("Starting SickChill using config")sickchill/oldbeard/helpers.py (16)
Line range hint
54-62
: Consider the implications of globally disabling insecure request warnings.Disabling
InsecureRequestWarning
might hide important security notices. It's generally safer to handle these warnings more granularly or enable them by default.
Line range hint
84-101
: Document the in-place modification of XML elements.It would be beneficial to note in the function's documentation that
indentXML
modifies the XML elements in place, as this might not be expected by all users.
Line range hint
103-152
: Optimize regex usage by compiling patterns ahead of time.Compiling regex patterns once and storing them for reuse can significantly improve performance, especially if
remove_non_release_groups
is called frequently.+ compiled_patterns = {pattern: re.compile(rf"(?i){pattern}") for pattern in removeWordsList.keys()} - _name = re.sub(rf"(?i){remove_string}", "", _name) + _name = compiled_patterns[remove_string].sub("", _name)
Line range hint
154-166
: Enhance error logging inmakeDir
function.Include more specific details in the logging statements when an exception occurs in
os.makedirs
, to aid in troubleshooting.- except OSError: + except OSError as e: + logger.error(f"Failed to create directory {path}: {str(e)}")
Line range hint
168-186
: Refactorlist_media_files
to useos.walk
for improved performance.Using
os.walk
can simplify the code and improve performance by handling the recursion internally:- for entry in os.listdir(path): - full_entry = os.path.join(path, entry) - if os.path.isdir(full_entry) and not entry.startswith(".") and not entry == "Extras": - files += list_media_files(full_entry) - elif is_media_file(entry): - files.append(full_entry) + for dirpath, dirnames, filenames in os.walk(path): + dirnames[:] = [d for d in dirnames if not d.startswith(".") and d != "Extras"] + files.extend(os.path.join(dirpath, f) for f in filenames if is_media_file(f))
Line range hint
188-210
: Adjust the logging level for exception stack traces incopyFile
.Consider logging the stack trace at the debug level to reduce log verbosity in production environments.
- logger.exception(f"There was a problem copying a file from {srcFile} to {destFile}. Error: {error}") + logger.error(f"There was a problem copying a file from {srcFile} to {destFile}. Error: {error}") + logger.debug(traceback.format_exc())
Line range hint
212-228
: Document behavior ofmoveFile
across different filesystems.It would be helpful to document that
moveFile
may fall back to copying and deleting the source file if moving across different filesystems fails.
Line range hint
230-248
: Check filesystem support for hard links inhardlinkFile
.Before attempting to create a hard link, check if the filesystem supports it to avoid unnecessary operations and errors.
+ if not os.path.supports_hard_links(srcFile): + logger.warning("Filesystem does not support hard links. Copying instead.") + copyFile(srcFile, destFile) + return
Line range hint
250-266
: Enhance error messages inmoveAndSymlinkFile
for better troubleshooting.Provide more detailed error messages to help identify the cause of failures when moving files or creating symbolic links.
- logger.warning(_("There was a problem creating a symlink of {srcFile} at {destFile}. Copying instead. Error: {error}").format(srcFile=srcFile, destFile=destFile, error=error)) + logger.warning(f"There was a problem creating a symlink of {srcFile} at {destFile}. Attempting to copy instead. Error: {error}")
Line range hint
268-308
: Optimize directory existence checks inmake_dirs
.Reduce the number of
os.path.isdir
calls to optimize performance, especially when creating multiple directories.- if os.path.isdir(sofar): + if sofar in existing_dirs: + continue + existing_dirs.add(sofar)
Line range hint
310-344
: Enhance error handling for file system permissions inrename_ep_file
.Improve error handling to specifically address file system permissions issues, which can commonly occur during file renaming and directory deletion.
- except (OSError, IOError) as error: + except PermissionError as error: + logger.error(f"Permission denied when renaming {cur_path} to {new_path}. Error: {error}") + except (OSError, IOError) as error:
Line range hint
346-376
: Optimize directory listing checks indelete_empty_folders
.Reduce the number of
os.listdir
calls to optimize performance, especially when cleaning up multiple directories.- check_files = os.listdir(check_empty_dir) + if check_empty_dir in checked_dirs: + continue + checked_dirs.add(check_empty_dir)
Line range hint
378-390
: Document the significance of each bit infileBitFilter
.Provide detailed documentation for each bit being removed by
fileBitFilter
to help users understand the impact on file permissions.
Line range hint
392-428
: Cache parent permissions inchmodAsParent
to optimize performance.Reduce the number of
os.stat
calls by caching the permissions of parent paths, especially when processing multiple files in the same directory.- parentPathStat = os.stat(parentPath) + if parentPath not in cached_permissions: + cached_permissions[parentPath] = os.stat(parentPath) + parentPathStat = cached_permissions[parentPath]
Line range hint
430-466
: Enhance error handling for permission issues infixSetGroupID
.Improve error handling to specifically address permission issues, which can commonly occur when setting group IDs on filesystems.
- except (OSError, PermissionError) as error: + except PermissionError as error: + logger.error(f"Permission denied when setting group ID on {childPath}. Error: {error}") + except OSError as error:
Line range hint
468-490
: Enhance error handling for database query issues inget_absolute_number_from_season_and_episode
.Improve error handling to specifically address issues that can occur during database queries, such as connection errors or query syntax errors.
- sql_results = main_db_con.select(sql, [show.indexerid, season, episode]) + try: + sql_results = main_db_con.select(sql, [show.indexerid, season, episode]) + except Exception as e: + logger.error(f"Database query failed for show {show.name}. Error: {e}") + return None
get_body_arguements for saveGeneral config.
This is part 1 - will rearrange mako logging to it's own part to consolidate logging items as part 2.
Then move onto other parts of config.
Merged in editshow return fix.
combined all arg works
Summary by CodeRabbit
Refactor
Bug Fixes
check_url
function for improved request handling.handle_requests_exception
function for increased robustness.Chores
logger
import inlibnotify.py
to augment logging capabilities.[tool.ruff]
configuration section inpyproject.toml
withline-length = 160
setting.